-
Notifications
You must be signed in to change notification settings - Fork 556
Start documenting tests/rustdoc-json #2422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start documenting tests/rustdoc-json #2422
Conversation
27c7a0d
to
abd8c05
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! r=me with or without addressing the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you link to here from https://rustc-dev-guide.rust-lang.org/rustdoc.html#tests ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in #2298 (which added this page as a stub, and linked to it).
…illaumeGomez jsondocck: Refactor directive handling Best reviewed commit by commit. 1. Moves directive handling into its own file. This makes it easier to link to in the dev-guide (rust-lang/rustc-dev-guide#2422 (comment)), but also makes the code nicer in it's own right 2. Renames command to directive. This is what compiletest uses, and it's nice to not have 2 words for this. r? `@GuillaumeGomez`
…illaumeGomez jsondocck: Refactor directive handling Best reviewed commit by commit. 1. Moves directive handling into its own file. This makes it easier to link to in the dev-guide (rust-lang/rustc-dev-guide#2422 (comment)), but also makes the code nicer in it's own right 2. Renames command to directive. This is what compiletest uses, and it's nice to not have 2 words for this. r? ``@GuillaumeGomez``
Rollup merge of #141709 - aDotInTheVoid:split-for-docs, r=GuillaumeGomez jsondocck: Refactor directive handling Best reviewed commit by commit. 1. Moves directive handling into its own file. This makes it easier to link to in the dev-guide (rust-lang/rustc-dev-guide#2422 (comment)), but also makes the code nicer in it's own right 2. Renames command to directive. This is what compiletest uses, and it's nice to not have 2 words for this. r? ``@GuillaumeGomez``
jsondocck: Refactor directive handling Best reviewed commit by commit. 1. Moves directive handling into its own file. This makes it easier to link to in the dev-guide (rust-lang/rustc-dev-guide#2422 (comment)), but also makes the code nicer in it's own right 2. Renames command to directive. This is what compiletest uses, and it's nice to not have 2 words for this. r? ``@GuillaumeGomez``
jsondocck: Refactor directive handling Best reviewed commit by commit. 1. Moves directive handling into its own file. This makes it easier to link to in the dev-guide (#2422 (comment)), but also makes the code nicer in it's own right 2. Renames command to directive. This is what compiletest uses, and it's nice to not have 2 words for this. r? ``@GuillaumeGomez``
@aDotInTheVoid it's been a bit—how do you feel about merging as-is and making improvements in follow-up PRs? |
abd8c05
to
0fd4c18
Compare
Thanks for the PR. If you have write access, feel free to merge this PR if it does not need reviews. You can request a review using |
Filled out a bit more stuff, I think it's mergable now, but still don't love it. It seems to focused on how the test-infra works, and not how to write tests. CC @obi1kenobi @fmease @jdonszelmann, would appreciate feedback on if this is clear/what's missing. |
I'm gonna merge this, it's Good Enough (tm), and improvements can be made in follow-ups. |
Sorry, didn't get to review it, I'll check out the chapter later |
Retroactive LGTM from me too. If you're considering making further improvements from here, I think the biggest improvement I can think of is to document all the things that are known to not currently work, linking to their respective issues. Things like "query selectors that go more than one level deep" etc. that are unintuitive and would burn a new contributor — precisely the person who would find this document most useful already :) |
Thanks, much apprecieated.
This was fixed in rust-lang/rust#138763! (And part of the reason this happened now, because I don't need to document all the workarounds to the broken old parser. Are there any other footguns you're aware of? I think we've got all of them, but could easily be wrong. |
Ah, wonderful news! The issues I was thinking of might have been fixed then, so I'll just keep my eyes open and update that doc if I come across anything else. |
Closes rust-lang/rust#100515
Rendered